Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Fake IDE/RPC Debugger Demo #414

Conversation

JonahSussman
Copy link
Contributor

The commit history looks ugly, I just need to rebase

@JonahSussman JonahSussman force-pushed the initial-rpc-server branch 2 times, most recently from e0a2be2 to 958572d Compare October 9, 2024 18:55
@JonahSussman JonahSussman changed the title ✨ Fake IDE <-> RPC Server Demo ✨ Fake IDE/RPC Debugger Demo Oct 9, 2024
@JonahSussman JonahSussman marked this pull request as ready for review October 9, 2024 18:57
process_id: Optional[int]

root_uri: str
kantra_uri: str
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need kantra_uri anymore

Copy link
Contributor

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we need to make some changes to the code plan loop, and we need to finish the getRagSolution piece.

Overall this looks really good, albeit a large change.

playpen/middleman/server.py Outdated Show resolved Hide resolved
playpen/middleman/server.py Show resolved Hide resolved
playpen/middleman/server.py Outdated Show resolved Hide resolved
playpen/middleman/server.py Show resolved Hide resolved
playpen/middleman/server.py Outdated Show resolved Hide resolved
@@ -43,13 +48,13 @@ def __eq__(self, o: object) -> bool:
return True
return False

def __iter__(self) -> Iterator[str]:
return iter([v.to_dict() for _, v in self.items()])
# def __iter__(self) -> Iterator[str]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this gross? I needed to convert nested objects that are a dict to a list instead as I didn't care about their keys

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix typo

Copy link
Contributor

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, only thing I would worry about before hitting merge is the reflection agent not being there.

As we don't have any tests to tell us if it is changing results or not, I would leave as it was and we can come back to this.

kai/service/llm_interfacing/model_provider.py Outdated Show resolved Hide resolved
playpen/middleman/server.py Show resolved Hide resolved
playpen/middleman/server.py Show resolved Hide resolved
MavenCompileStep(task_manager_config),
AnalyzerLSPStep(task_manager_config),
],
agents=[
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are going to want to add the dependency agents

note to selves, rename to task runners

@@ -7,6 +7,12 @@
@dataclass(eq=False, kw_only=True)
class AnalyzerRuleViolation(ValidationError):
incident: Incident

# NOTE(JonahSussman): Violation contains a list of Incidents, and RuleSet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a plus one on this comment and doing it in a followup

@@ -13,59 +13,66 @@
from playpen.repo_level_awareness.task_runner.analyzer_lsp.api import (
AnalyzerRuleViolation,
)
from playpen.rpc.core import JsonRpcServer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! thanks for updating this, I was planning on coming back to this but this is great!

playpen/repo_level_awareness/vfs/git_vfs.py Outdated Show resolved Hide resolved
Signed-off-by: JonahSussman <[email protected]>
Signed-off-by: JonahSussman <[email protected]>
Signed-off-by: JonahSussman <[email protected]>
Signed-off-by: JonahSussman <[email protected]>
Signed-off-by: JonahSussman <[email protected]>
Signed-off-by: JonahSussman <[email protected]>
Signed-off-by: JonahSussman <[email protected]>
Signed-off-by: JonahSussman <[email protected]>
Signed-off-by: JonahSussman <[email protected]>
Signed-off-by: JonahSussman <[email protected]>
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman JonahSussman merged commit c36aabc into konveyor:feature/repository-level-awareness Oct 16, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants